Revert "fix: set taxes and totals before validating subcontracting transaction" by karm1000 · Pull Request #3783 · resilient-tech/india-compliance

karm1000 · 2025-11-11T07:44:51Z

Reverts #3782

Summary by CodeRabbit

Bug Fixes
- Fixed GST tax calculation initialization for subcontracting transactions to ensure tax data is properly updated during validation.
- Corrected tax value handling for unregistered company subcontracting scenarios in GST compliance.

…ansaction"

coderabbitai · 2025-11-11T07:45:01Z

Walkthrough

This PR reintroduces tax and totals initialization for GST subcontracting transactions by invoking CustomTaxController within the validate() method. Test expectations are updated to reflect the new tax-handling behavior where total_taxes is now None instead of 0.0, and a fixture value is adjusted accordingly.

Changes

Cohort / File(s)	Summary
Subcontracting Transaction Override and Tests `india_compliance/gst_india/overrides/subcontracting_transaction.py`, `india_compliance/gst_india/overrides/test_subcontracting_transaction.py`	Reintroduces `CustomTaxController(doc, field_map).set_taxes_and_totals()` call in `validate()` method after validation checks, with field_map selection based on doctype (Stock Entry vs Subcontracting Order Receipt); test assertions updated to expect `total_taxes` as `None` instead of `0.0` for unregistered company scenarios.
ITC-04 Export Test Fixture `india_compliance/gst_india/utils/itc_04/test_itc_04_export.py`	Test fixture `txval` adjusted from `200.0` to `0.0` for an item in the expected JSON payload.

Sequence Diagram

sequenceDiagram
    participant validate as validate()
    participant igv as ignore_gst_validations<br/>_for_subcontracting check
    participant txn as Transaction Name<br/>Validation
    participant tax as CustomTaxController
    participant sub as Subsequent GST<br/>Validations

    validate->>igv: Check if should skip GST
    alt Skip GST Validations
        igv-->>validate: Skip tax init
    else Process GST
        igv-->>validate: Continue
        validate->>txn: Validate transaction name
        txn-->>validate: Name OK
        validate->>tax: set_taxes_and_totals()<br/>(with field_map)
        note over tax: Initializes tax/totals<br/>for Stock Entry or<br/>Subcontracting Order Receipt
        tax-->>validate: Taxes set
        validate->>sub: Run GST validations
        sub-->>validate: Validations complete
    end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Focused changes to validation flow in a single override file
Test expectation updates are straightforward assertions
Verify that the field_map selection logic correctly handles both Stock Entry and Subcontracting Order Receipt scenarios
Confirm tax initialization timing doesn't conflict with subsequent GST validations

Possibly related PRs

fix: set taxes and totals before validating subcontracting transaction #3782: Modifies tax/totals initialization calls in the same subcontracting_transaction.py validate() method, affecting when and how CustomTaxController sets taxes.

Suggested labels

squash

Suggested reviewers

ljain112
vorasmit

Poem

🐰 A rabbit hops through GST code so neat,
Where taxes flow and totals meet,
Validation checks align in place,
Field maps guide the tax embrace,
Compliance blooms in every trace! 🌱

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title clearly and specifically describes the main change: reverting a previous fix for tax and totals handling in subcontracting transactions, which aligns with the summary showing reintroduction and modification of tax initialization logic.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch revert-3782-set-values-even-if-not-eligible

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1f2d9f7 and e201369.

📒 Files selected for processing (3)

india_compliance/gst_india/overrides/subcontracting_transaction.py (1 hunks)
india_compliance/gst_india/overrides/test_subcontracting_transaction.py (2 hunks)
india_compliance/gst_india/utils/itc_04/test_itc_04_export.py (1 hunks)

🧰 Additional context used

🧠 Learnings (6)

📓 Common learnings

Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3399
File: india_compliance/gst_india/utils/gstr_1/test_gstr_1_books_data.py:791-822
Timestamp: 2025-05-29T15:22:04.761Z
Learning: In the india_compliance test suite, the IntegrationTestCase framework automatically handles database rollbacks after test completion, which means functions that modify global state (like GST Settings and Item Tax Templates) in setUpClass won't persist beyond the test run and don't require manual cleanup.

📚 Learning: 2025-06-25T08:19:02.607Z

Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3354
File: india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py:278-281
Timestamp: 2025-06-25T08:19:02.607Z
Learning: In the Summary of ITC Availed report (india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py), the summary dictionaries created by get_initial_summary() are never empty - they always contain tax field keys (igst_amount, cgst_amount, sgst_amount, cess_amount) with 0 values, so checking for empty dictionaries is unnecessary.

Applied to files:

india_compliance/gst_india/overrides/subcontracting_transaction.py
india_compliance/gst_india/utils/itc_04/test_itc_04_export.py
india_compliance/gst_india/overrides/test_subcontracting_transaction.py

📚 Learning: 2025-10-01T10:54:11.096Z

Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3709
File: india_compliance/gst_india/utils/exporter.py:174-174
Timestamp: 2025-10-01T10:54:11.096Z
Learning: In india_compliance/gst_india/doctype/gstr_1/gstr_1_export.py, FIELD_TRANSFORMATIONS dictionary contains lambdas that are only called internally by DataProcessor.apply_transformations() with a single argument. These are separate from header transforms used with ExcelExporter.insert_data() which accept two arguments (value, row).

Applied to files:

india_compliance/gst_india/overrides/subcontracting_transaction.py

📚 Learning: 2025-04-25T10:12:24.584Z

Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3344
File: india_compliance/gst_india/report/account_wise_summary/account_wise_summary.py:81-87
Timestamp: 2025-04-25T10:12:24.584Z
Learning: When processing taxes in GST India reports, charges after GST rows with charge_type "On Previous Row Total" should not be used to compute taxable value, so a break statement is used to exit the tax processing loop once such a row is encountered.

Applied to files:

india_compliance/gst_india/overrides/subcontracting_transaction.py

📚 Learning: 2025-06-25T08:16:18.010Z

Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3354
File: india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py:309-310
Timestamp: 2025-06-25T08:16:18.010Z
Learning: In the Summary of ITC Availed report (india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py), creating both a main entry and an additional indented entry for categories without subcategories is by design and intentional behavior for the report structure.

Applied to files:

india_compliance/gst_india/overrides/subcontracting_transaction.py

📚 Learning: 2025-05-29T15:22:04.761Z

Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3399
File: india_compliance/gst_india/utils/gstr_1/test_gstr_1_books_data.py:791-822
Timestamp: 2025-05-29T15:22:04.761Z
Learning: In the india_compliance test suite, the IntegrationTestCase framework automatically handles database rollbacks after test completion, which means functions that modify global state (like GST Settings and Item Tax Templates) in setUpClass won't persist beyond the test run and don't require manual cleanup.

Applied to files:

india_compliance/gst_india/overrides/test_subcontracting_transaction.py

🧬 Code graph analysis (1)

india_compliance/gst_india/overrides/subcontracting_transaction.py (1)

india_compliance/gst_india/utils/taxes_controller.py (2)

CustomTaxController (55-211)

set_taxes_and_totals (69-73)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: Codacy Static Code Analysis
GitHub Check: Python Unit Tests
GitHub Check: Mergify Merge Protections
GitHub Check: Summary

🔇 Additional comments (4)

india_compliance/gst_india/overrides/subcontracting_transaction.py (1)

190-195: LGTM! Field map selection and tax controller invocation are correctly implemented.

The logic properly selects the appropriate field map based on document type and invokes CustomTaxController to initialize taxes and totals. The placement after the early-return validation checks ensures this only executes for applicable e-waybill transactions.

india_compliance/gst_india/overrides/test_subcontracting_transaction.py (2)

254-254: LGTM! Test expectation correctly updated.

The assertion now expects total_taxes to be None for unregistered company scenarios. This aligns with the reintroduced validation flow where ignore_gst_validations_for_subcontracting() returns True for unregistered companies, causing an early return before CustomTaxController.set_taxes_and_totals() is invoked.

274-274: LGTM! Test expectation correctly updated.

The assertion now expects total_taxes to be None for material receipt stock entries. This is correct because "Material Receipt" is not in the list of e-waybill-applicable purposes (lines 522-526 in subcontracting_transaction.py), causing is_e_waybill_applicable() to return False and validate() to return early before taxes are calculated.

india_compliance/gst_india/utils/itc_04/test_itc_04_export.py (1)

58-58: No verification needed—the expected behavior is correct.

The txval: 0.0 in the test at line 58 is the correct and intended value for raw material stock entries in ITC-04 exports. Stock entries represent internal material transfers to job workers, not taxable supplies. They should have zero taxable value. The test explicitly validates this expected response, confirming the behavior is by design.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Revert "fix: set taxes and totals before validating subcontracting tr…

e201369

…ansaction"

karm1000 closed this Nov 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "fix: set taxes and totals before validating subcontracting transaction"#3783

Revert "fix: set taxes and totals before validating subcontracting transaction"#3783
karm1000 wants to merge 1 commit intodevelopfrom
revert-3782-set-values-even-if-not-eligible

karm1000 commented Nov 11, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Nov 11, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

karm1000 commented Nov 11, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

karm1000 commented Nov 11, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Nov 11, 2025 •

edited

Loading